-
Notifications
You must be signed in to change notification settings - Fork 33
CLI (feat): Adds ephemeral seed usage for QOL and additional commands #416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@joncinque tagging from discord |
joncinque
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay on the review -- really great work! Mostly small things, then we can land this. The semver failure is unrelated
| // Account exists - check if it's initialized (has non-zero data) | ||
| if account.data.iter().all(|&x| x == 0) { | ||
| // Account exists but is uninitialized, can use this seed | ||
| return Ok(seed); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this check is sufficient -- the program tries to allocate and assign the ephemeral stake without checking its prior state, so this check should also include account.owner == system_program::id()
| let ephemeral_stake_seed = match ephemeral_stake_seed { | ||
| Some(seed) => seed, | ||
| None => { | ||
| // Find an unused ephemeral stake seed | ||
| find_unused_ephemeral_stake_seed( | ||
| &config.rpc_client, | ||
| &config.stake_pool_program_id, | ||
| stake_pool_address, | ||
| u64::MAX, // Check all possible ephemeral seeds | ||
| )? | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: probably better to do this after the update, in case any accounts get cleaned up in the process
| let ephemeral_stake_seed = match ephemeral_stake_seed { | ||
| Some(seed) => seed, | ||
| None => { | ||
| // Find an unused ephemeral stake seed | ||
| find_unused_ephemeral_stake_seed( | ||
| &config.rpc_client, | ||
| &config.stake_pool_program_id, | ||
| stake_pool_address, | ||
| u64::MAX, // Check all possible ephemeral seeds | ||
| )? | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: same here, probably better to do it after updating
| .arg( | ||
| Arg::with_name("allow_additional") | ||
| .long("allow-additional") | ||
| .short("aa") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: let's avoid two-letter short flags and remove this entirely
| .long("allow-additional") | ||
| .short("aa") | ||
| .takes_value(false) | ||
| .help("Allow automatic fallback to additional validator stake with ephemeral accounts if transient stake is in use"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit for clarity
| .help("Allow automatic fallback to additional validator stake with ephemeral accounts if transient stake is in use"), | |
| .help("Allow automatic fallback to increase-additional-validator-stake with ephemeral accounts if transient stake is in use"), |
| .arg( | ||
| Arg::with_name("allow_additional") | ||
| .long("allow-additional") | ||
| .short("aa") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: same here, no two-letter short flags, please
| .long("allow-additional") | ||
| .short("aa") | ||
| .takes_value(false) | ||
| .help("Allow automatic fallback to additional validator stake with ephemeral accounts if transient stake is in use"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: same here for clarity
| .help("Allow automatic fallback to additional validator stake with ephemeral accounts if transient stake is in use"), | |
| .help("Allow automatic fallback to decrease-additional-validator-stake with ephemeral accounts if transient stake is in use"), |
Added intelligent fallback logic and dedicated commands to handle additional validator stake operations when transient accounts are in use, making stake management more user-friendly and automated.
increase-validator-stakeanddecrease-validator-stakecommands with--allow-additionalflagincrease-additional-validator-stakeDirectly increase stake using ephemeral accounts with optional--ephemeral-seedflagdecrease-additional-validator-stakeDirectly decrease stake using ephemeral accounts with optional--ephemeral-seedflagfind_unused_ephemeral_stake_seedhelper to find next-available ephemeral stake accounts via brute force iterationUsage: